feat: add agent session sync - #48
Conversation
📝 WalkthroughWalkthroughThis PR introduces "Agent Sync," a feature bridging Codex and Claude Code session history into Hebb Mind. It adds transcript multi-turn extraction, a session discovery/normalization module, a FastAPI router with sessions/sync endpoints, a ChangesAgent Sync Feature
CLI Source-Checkout Command Resolution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WebConsole
participant CLI
participant AgentSyncRouter
participant SessionSync
participant MemoryStore
participant Embedder
WebConsole->>AgentSyncRouter: GET /api/v1/agent-sync/sessions
CLI->>AgentSyncRouter: POST /api/v1/agent-sync/sync
AgentSyncRouter->>SessionSync: discover_sessions(host, limit)
AgentSyncRouter->>MemoryStore: fetch existing turn keys
AgentSyncRouter->>Embedder: batch embed pending turns
AgentSyncRouter->>MemoryStore: store new memories
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Agent Sync' feature, establishing Hebb Mind as a shared memory hub for Claude Code and Codex by parsing, collecting, and syncing local session histories into the database. It adds corresponding Web Console pages, a new hebb agent-sync CLI command group, and server API endpoints. The review feedback highlights several critical performance and robustness improvements, such as offloading synchronous file I/O to a thread pool to avoid blocking the FastAPI event loop, chunking large embedding requests to prevent OOM errors, adding defensive checks for missing metadata or directories, and formatting raw epoch timestamps into human-readable dates in the CLI.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| store: MemoryStore = Depends(get_memory_store), | ||
| ) -> list[AgentSessionOut]: | ||
| """List local Codex and Claude Code sessions with sync status.""" | ||
| sessions = session_sync.discover_sessions(host=host, limit=limit) |
There was a problem hiding this comment.
The session_sync.discover_sessions function performs synchronous file I/O (directory scanning and file reading). Calling it directly inside an async def path operation blocks the FastAPI event loop, which can severely degrade performance under concurrent load. We should run it in an external thread pool using asyncio.to_thread.run.
| sessions = session_sync.discover_sessions(host=host, limit=limit) | |
| import asyncio | |
| sessions = await asyncio.to_thread.run(session_sync.discover_sessions, host, limit) |
| embedder: EmbeddingProvider = Depends(get_embedder), | ||
| ) -> AgentSyncResponse: | ||
| """Sync local Codex and Claude Code session turns into Hebb Mind.""" | ||
| sessions = session_sync.discover_sessions(host=request.host, limit=request.limit) |
There was a problem hiding this comment.
Similar to list_sessions, session_sync.discover_sessions is a synchronous I/O-bound function and should be run in a thread pool using asyncio.to_thread.run to avoid blocking the FastAPI event loop.
import asyncio
sessions = await asyncio.to_thread.run(session_sync.discover_sessions, request.host, request.limit)| embeddings = await embedder.embed_batch([memory.content for _, _, memory in pending]) | ||
| if len(embeddings) != len(pending): | ||
| raise HTTPException( | ||
| status_code=502, | ||
| detail=f"Embedder returned {len(embeddings)} vectors for {len(pending)} imported turns", | ||
| ) |
There was a problem hiding this comment.
Generating embeddings for all pending turns in a single batch can lead to Out-Of-Memory (OOM) errors or exceed API payload/rate limits if the history is large. We should chunk the pending list and generate embeddings in smaller batches (e.g., 128 items at a time).
batch_size = 128
embeddings = []
for i in range(0, len(pending), batch_size):
batch = pending[i : i + batch_size]
batch_embeddings = await embedder.embed_batch([memory.content for _, _, memory in batch])
if len(batch_embeddings) != len(batch):
raise HTTPException(
status_code=502,
detail=f"Embedder returned {len(batch_embeddings)} vectors for {len(batch)} imported turns",
)
embeddings.extend(batch_embeddings)| def _metadata_dict(memory: Memory) -> dict[str, object]: | ||
| return memory.metadata.model_dump(exclude_none=True) |
There was a problem hiding this comment.
If a memory in the database does not have any metadata (i.e., memory.metadata is None), calling model_dump() will raise an AttributeError. We should add a defensive check to return an empty dictionary if metadata is None.
| def _metadata_dict(memory: Memory) -> dict[str, object]: | |
| return memory.metadata.model_dump(exclude_none=True) | |
| def _metadata_dict(memory: Memory) -> dict[str, object]: | |
| if memory.metadata is None: | |
| return {} | |
| return memory.metadata.model_dump(exclude_none=True) |
| def _codex_session_paths() -> list[Path]: | ||
| home = _codex_home() | ||
| candidates: list[Path] = [] |
There was a problem hiding this comment.
For consistency with _claude_session_paths(), we should add a defensive check to ensure that the home directory exists and is indeed a directory before attempting to glob files. This prevents potential issues if the directory does not exist.
def _codex_session_paths() -> list[Path]:
home = _codex_home()
if not home.is_dir():
return []
candidates: list[Path] = []| for session in sessions: | ||
| turn_count = int(session.get("turn_count") or 0) | ||
| synced = int(session.get("synced_turns") or 0) | ||
| pending = int(session.get("unsynced_turns") or 0) | ||
| table.add_row( | ||
| _host_label(str(session.get("host") or "")), | ||
| str(session.get("project") or "-"), | ||
| f"{synced}/{turn_count}", | ||
| str(pending), | ||
| str(session.get("latest_timestamp") or session.get("updated_at") or "-"), | ||
| str(session.get("id") or "-"), | ||
| ) |
There was a problem hiding this comment.
If latest_timestamp is missing, the table falls back to updated_at, which is a raw float (epoch seconds). Printing raw floats in a CLI table is not user-friendly. We should format it into a human-readable date string.
for session in sessions:
turn_count = int(session.get("turn_count") or 0)
synced = int(session.get("synced_turns") or 0)
pending = int(session.get("unsynced_turns") or 0)
updated_val = session.get("latest_timestamp")
if not updated_val and session.get("updated_at"):
from datetime import datetime
try:
updated_val = datetime.fromtimestamp(float(session["updated_at"])).strftime("%Y-%m-%d %H:%M")
except (ValueError, TypeError):
updated_val = str(session["updated_at"])
table.add_row(
_host_label(str(session.get("host") or "")),
str(session.get("project") or "-"),
f"{synced}/{turn_count}",
str(pending),
str(updated_val or "-"),
str(session.get("id") or "-"),
)![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
src/hebb/integrations/claude_code/transcript.py (1)
140-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
Raisesdocstring section onextract_turns.
extract_turnscalls_load_main_messages, which is documented to raiseOSError, without catching it — so the exception propagates to callers. The docstring only hasArgs/Returns, unlike_load_main_messagesitself and the siblingextract_turnsincodex/transcript.py, which both documentRaises: OSError. Callers of this API (e.g.session_sync._parse_turns) currently handleOSError, but the contract should be documented here too.📝 Proposed docstring fix
Returns: Parsed turn records in transcript order. Low-signal user prompts and incomplete turns without assistant output are omitted. + + Raises: + OSError: If the transcript file exists but cannot be read. """As per coding guidelines: "Include docstring with Args, Returns, and Raises sections for all public APIs."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/integrations/claude_code/transcript.py` around lines 140 - 198, Add a Raises section to the public API docstring for extract_turns to document that OSError can propagate from _load_main_messages. Update the extract_turns docstring alongside Args and Returns so it matches the contract used by _load_main_messages and the sibling extract_turns in codex/transcript.py, without changing the function logic.Source: Coding guidelines
tests/integration/server/test_agent_sync_router.py (1)
43-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
/tmp/repoinstead oftmp_path.The sibling test file (
test_agent_session_sync.py) usesstr(tmp_path / "repo")for the same field; this test hardcodes"/tmp/repo"instead.🧹 Proposed fix
"payload": {"id": "session-a", "cwd": "/tmp/repo"}, + "payload": {"id": "session-a", "cwd": str(home / "repo")},As per coding guidelines: "MUST NOT hardcode API keys, secrets, or absolute paths outside the user's workspace."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/server/test_agent_sync_router.py` around lines 43 - 62, The session fixture in _write_codex_session hardcodes the cwd payload to an absolute /tmp/repo path, which should be replaced with the test’s temporary workspace path. Update the session_meta payload in test_agent_sync_router.py to use the provided tmp_path-based repo location, matching the approach used in test_agent_session_sync.py and keeping the cwd inside the user workspace.Sources: Coding guidelines, Linters/SAST tools
src/hebb/server/routers/agent_sync.py (2)
135-146: 🚀 Performance & Scalability | 🔵 TrivialFull hippocampus partition scan on every
/sessionsand/synccall.
_existing_turn_keysloads and iterates all memories inHIPPOCAMPUS_PARTITIONon each request, including theGET /sessionslist endpoint that the web console likely polls. This will get slower as synced memories accumulate.Consider an indexed/queryable lookup by
(host, session_id, turn)metadata (e.g. a store-level filter) instead of a full partition materialization, if partition sizes are expected to grow large.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/server/routers/agent_sync.py` around lines 135 - 146, The `_existing_turn_keys` helper is doing a full `HIPPOCAMPUS_PARTITION` scan via `store.get_by_partition` on every `/sessions` and `/sync` request, which will not scale as memories grow. Update this path to use a more targeted lookup in `MemoryStore` based on the `(host, session_id, turn)` metadata, ideally by adding or reusing a store-level filter/indexed query instead of materializing the entire partition. Keep the existing `session_sync.turn_key` and `_metadata_dict` flow, but change `_existing_turn_keys` to retrieve only matching records rather than iterating all memories.
91-97: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDedup check doesn't guard against in-batch collisions across sessions.
existingis only updated after a turn is actually persisted (line 130), not when it's added topendinghere. If two discoveredAgentSessions ever share the same(host, session_id, turn)(e.g. a transcript split across two files but keeping the same session id), both entries pass this check and both get queued and persisted, defeating the dedup purpose designed byturn_key.♻️ Proposed fix
for session in sessions: skipped = 0 for turn in session.turns: if _has_existing(existing, session.host, session.session_id, turn.turn): skipped += 1 continue + existing.add(session_sync.turn_key(session.host, session.session_id, turn.turn)) pending.append((session, turn, session_sync.to_memory_create(session, turn)))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/server/routers/agent_sync.py` around lines 91 - 97, The dedup logic in the session turn collection loop does not prevent collisions within the same batch because `existing` is only updated after persistence, so duplicate `(host, session_id, turn)` entries from different `AgentSession`s can both be queued. Update the `pending`-building flow in `agent_sync` to mark each accepted turn as reserved immediately after `_has_existing` passes, using the same `turn_key`/`existing` tracking used later on persist, so later sessions in the same run will skip already-queued turns.src/hebb/static/css/style.css (1)
803-809: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeprecated
word-break: break-wordvalue.Stylelint flags this as deprecated. Prefer
overflow-wrap: anywherefor wrapping long titles/names.🎨 Proposed fix
.agent-flow-title, .agent-hub-name { font-size: 17px; font-weight: 700; color: var(--text-primary); - word-break: break-word; + overflow-wrap: anywhere; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/css/style.css` around lines 803 - 809, The title/name styling in the `.agent-flow-title` and `.agent-hub-name` rule uses the deprecated `word-break: break-word` value; update that CSS block to use `overflow-wrap: anywhere` instead so long labels still wrap correctly. Keep the change localized to the shared selector rule in the stylesheet and remove the deprecated property.Source: Linters/SAST tools
src/hebb/static/js/components/agent-sync.js (3)
61-73: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
load()fetches all sessions with nolimit.The API supports a
limit(max 500 per theAgentSyncRequest/query schema), butload()never passes one, so the console always requests the full unfiltered session list. Over a long local history this could grow unbounded and slow the initial render.⚡ Suggested fix
- sessions = await api.listAgentSessions(); + sessions = await api.listAgentSessions({ limit: 500 });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/agent-sync.js` around lines 61 - 73, The load() function in agent-sync.js fetches every agent session without any limit, which can make initial rendering slow as history grows. Update load() to call api.listAgentSessions() with an explicit limit value at or below the AgentSyncRequest maximum (500), and keep the existing loading/error handling in place so sessions are still assigned from the bounded result set.
75-89: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo in-flight guard on
sync(); buttons stay clickable during a sync.Neither the "Sync pending" button (line 225/247) nor the per-session "Sync" button (line 188) is disabled while a sync request is in flight —
renderBody()'ssyncAll.disabledcheck only accounts forloading(session discovery), not an activesync()call. Rapid clicks can fire overlappingPOST /syncrequests.🔒 Suggested fix: track a `syncing` flag
let hostFilter = ''; let sessions = []; let loading = false; +let syncing = false;async function sync(root, ids = []) { + if (syncing) return; + syncing = true; + renderBody(root); try { const resp = await api.syncAgentSessions({ host: hostFilter || null, ids, }); success(t('agent_sync.synced_ok', { created: resp.memories_created, skipped: resp.skipped_existing, })); await load(root); } catch (e) { error(`${t('agent_sync.sync_failed')}: ${e.message}`); + } finally { + syncing = false; } }- if (syncAll) syncAll.disabled = loading || pending === 0; + if (syncAll) syncAll.disabled = loading || syncing || pending === 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/agent-sync.js` around lines 75 - 89, The sync action in `sync()` has no in-flight guard, so `Sync pending` and per-session `Sync` can be clicked multiple times and trigger overlapping requests. Add a `syncing` state flag alongside the existing `loading` logic in `renderBody()` and set it around `api.syncAgentSessions()` in `sync()` so both the global and row-level sync controls are disabled while a sync is running, then clear it in a finally path after the request completes.
18-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared
esc()helper.config-section.jsalready exports this function, and other components import it; keeping a separate copy here just adds another duplicate sanitizer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/agent-sync.js` around lines 18 - 24, The agent-sync component currently defines its own esc() sanitizer instead of reusing the shared helper. Remove the local esc() implementation in agent-sync.js and import the exported esc() from config-section.js, following the same pattern used by the other components so there is a single source of truth for escaping.Sources: Learnings, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@repo_pages/api/cli.md`:
- Around line 122-133: The CLI docs currently advertise unsupported flags and
values for the agent-sync commands, which do not exist in `hebb agent-sync` or
`src/hebb/cli/commands/agent_sync.py`. Update the usage block and options table
to match the real command surface by removing `--limit`, `--id`, and the
explicit `all` host value unless they are implemented, and keep only the
supported flags (`--host`, `--dry-run`, `--json`, `--url`) in the docs.
Reference the `agent-sync list` and `agent-sync sync` command descriptions so
the text stays consistent with the actual CLI behavior.
In `@repo_pages/zh/api/cli.md`:
- Around line 119-131: The `hebb agent-sync` CLI docs are describing unsupported
`--limit` and `--id` options that are not parsed by `agent_sync.py`. Update the
usage examples and option table in the `agent-sync` section to remove those
flags unless you also add them to the actual command implementation. Keep the
documented options aligned with the real parser in
`src/hebb/cli/commands/agent_sync.py` and the related `list`/`sync` command
behavior.
In `@src/hebb/cli/commands/agent_sync.py`:
- Around line 204-226: _update _fail_request in agent_sync.py to be annotated as
NoReturn instead of None, since it always exits via SystemExit(1) and never
returns. Use the _fail_request helper name and its existing exception-handling
paths to update the return type, and keep the raise SystemExit(1) behavior so
mypy can correctly treat the except branch as terminating and stop flagging
sessions/result as possibly unbound._
In `@src/hebb/server/routers/agent_sync.py`:
- Around line 122-130: The persist path in the `agent_sync` loop silently
swallows exceptions from `store.create()`, so failures are impossible to
diagnose. Update the `try/except` around `store.create(memory,
embedding=embedding)` to log the exception with enough context (for example the
current session/turn identifiers) before incrementing `item.failed`; keep the
counter update and `continue`, but make sure the logger records the error path
in the same `agent_sync` flow.
In `@src/hebb/utils/cli_paths.py`:
- Around line 60-81: The Windows branch in _source_checkout_command currently
omits the PYTHONPATH override, so the source checkout import behavior differs
from the POSIX path; update the function to propagate PYTHONPATH for both
branches, likely by returning the command together with an env override and
adjusting callers accordingly. Make sure the docstring for
_source_checkout_command matches the actual behavior, and add tests that cover
the os.name == "nt" case so the checkout-import contract is verified on Windows
too.
---
Nitpick comments:
In `@src/hebb/integrations/claude_code/transcript.py`:
- Around line 140-198: Add a Raises section to the public API docstring for
extract_turns to document that OSError can propagate from _load_main_messages.
Update the extract_turns docstring alongside Args and Returns so it matches the
contract used by _load_main_messages and the sibling extract_turns in
codex/transcript.py, without changing the function logic.
In `@src/hebb/server/routers/agent_sync.py`:
- Around line 135-146: The `_existing_turn_keys` helper is doing a full
`HIPPOCAMPUS_PARTITION` scan via `store.get_by_partition` on every `/sessions`
and `/sync` request, which will not scale as memories grow. Update this path to
use a more targeted lookup in `MemoryStore` based on the `(host, session_id,
turn)` metadata, ideally by adding or reusing a store-level filter/indexed query
instead of materializing the entire partition. Keep the existing
`session_sync.turn_key` and `_metadata_dict` flow, but change
`_existing_turn_keys` to retrieve only matching records rather than iterating
all memories.
- Around line 91-97: The dedup logic in the session turn collection loop does
not prevent collisions within the same batch because `existing` is only updated
after persistence, so duplicate `(host, session_id, turn)` entries from
different `AgentSession`s can both be queued. Update the `pending`-building flow
in `agent_sync` to mark each accepted turn as reserved immediately after
`_has_existing` passes, using the same `turn_key`/`existing` tracking used later
on persist, so later sessions in the same run will skip already-queued turns.
In `@src/hebb/static/css/style.css`:
- Around line 803-809: The title/name styling in the `.agent-flow-title` and
`.agent-hub-name` rule uses the deprecated `word-break: break-word` value;
update that CSS block to use `overflow-wrap: anywhere` instead so long labels
still wrap correctly. Keep the change localized to the shared selector rule in
the stylesheet and remove the deprecated property.
In `@src/hebb/static/js/components/agent-sync.js`:
- Around line 61-73: The load() function in agent-sync.js fetches every agent
session without any limit, which can make initial rendering slow as history
grows. Update load() to call api.listAgentSessions() with an explicit limit
value at or below the AgentSyncRequest maximum (500), and keep the existing
loading/error handling in place so sessions are still assigned from the bounded
result set.
- Around line 75-89: The sync action in `sync()` has no in-flight guard, so
`Sync pending` and per-session `Sync` can be clicked multiple times and trigger
overlapping requests. Add a `syncing` state flag alongside the existing
`loading` logic in `renderBody()` and set it around `api.syncAgentSessions()` in
`sync()` so both the global and row-level sync controls are disabled while a
sync is running, then clear it in a finally path after the request completes.
- Around line 18-24: The agent-sync component currently defines its own esc()
sanitizer instead of reusing the shared helper. Remove the local esc()
implementation in agent-sync.js and import the exported esc() from
config-section.js, following the same pattern used by the other components so
there is a single source of truth for escaping.
In `@tests/integration/server/test_agent_sync_router.py`:
- Around line 43-62: The session fixture in _write_codex_session hardcodes the
cwd payload to an absolute /tmp/repo path, which should be replaced with the
test’s temporary workspace path. Update the session_meta payload in
test_agent_sync_router.py to use the provided tmp_path-based repo location,
matching the approach used in test_agent_session_sync.py and keeping the cwd
inside the user workspace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 213d2b3d-09ef-4fd6-9d15-ba0c890ac229
📒 Files selected for processing (36)
AGENTS.mdREADME.mdREADME_ZH.mdrepo_pages/.vitepress/config.mtsrepo_pages/api/cli.mdrepo_pages/guide/agent-sync.mdrepo_pages/guide/web-console.mdrepo_pages/index.mdrepo_pages/public/llms.txtrepo_pages/quick-start.mdrepo_pages/zh/api/cli.mdrepo_pages/zh/guide/agent-sync.mdrepo_pages/zh/guide/web-console.mdrepo_pages/zh/index.mdrepo_pages/zh/quick-start.mdreports/analysis/codex-claude-code-session-memory-analysis.mdreports/design/agent-session-sync-design.mdsrc/hebb/cli/commands/agent_sync.pysrc/hebb/cli/main.pysrc/hebb/integrations/claude_code/transcript.pysrc/hebb/integrations/codex/transcript.pysrc/hebb/integrations/session_sync.pysrc/hebb/server/app.pysrc/hebb/server/routers/agent_sync.pysrc/hebb/static/css/style.csssrc/hebb/static/index.htmlsrc/hebb/static/js/api.jssrc/hebb/static/js/app.jssrc/hebb/static/js/components/agent-sync.jssrc/hebb/static/js/i18n.jssrc/hebb/utils/cli_paths.pytests/integration/server/test_agent_sync_router.pytests/unit/cli/commands/test_agent_sync.pytests/unit/integrations/test_agent_session_sync.pytests/unit/integrations/test_claude_code_hooks.pytests/unit/utils/test_cli_paths.py
| hebb agent-sync list [--host all|claude-code|codex] [--limit 100] [--json] [--url URL] | ||
| hebb agent-sync sync [--host all|claude-code|codex] [--id SESSION_ID]... [--limit 100] [--dry-run] [--json] [--url URL] | ||
| ``` | ||
|
|
||
| | Option | Applies to | Description | | ||
| |--------|------------|-------------| | ||
| | `--host` | `list`, `sync` | Filter to one source. `claude-code` maps to the API host `claude_code`. | | ||
| | `--limit` | `list`, `sync` | Maximum sessions to scan. | | ||
| | `--id` | `sync` | Sync only specific opaque session ids returned by `list --json`. May be repeated. | | ||
| | `--dry-run` | `sync` | Report pending turns without writing memories. | | ||
| | `--json` | `list`, `sync` | Print the raw API payload for scripts. | | ||
| | `--url` | `list`, `sync` | Override the server URL, useful for dev servers on non-default ports. | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove unsupported CLI flags from the docs.
The usage block/table advertises --limit, --id, and an explicit all host value, but src/hebb/cli/commands/agent_sync.py only exposes --host, --dry-run, --json, and --url. As written, users will copy flags that the command rejects. If these options are meant to ship, wire them through the CLI first; otherwise trim the docs to the real surface. To target all sessions today, omit --host.
Suggested doc correction
-hebb agent-sync list [--host all|claude-code|codex] [--limit 100] [--json] [--url URL]
-hebb agent-sync sync [--host all|claude-code|codex] [--id SESSION_ID]... [--limit 100] [--dry-run] [--json] [--url URL]
+hebb agent-sync list [--host claude-code|codex] [--json] [--url URL]
+hebb agent-sync sync [--host claude-code|codex] [--dry-run] [--json] [--url URL]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@repo_pages/api/cli.md` around lines 122 - 133, The CLI docs currently
advertise unsupported flags and values for the agent-sync commands, which do not
exist in `hebb agent-sync` or `src/hebb/cli/commands/agent_sync.py`. Update the
usage block and options table to match the real command surface by removing
`--limit`, `--id`, and the explicit `all` host value unless they are
implemented, and keep only the supported flags (`--host`, `--dry-run`, `--json`,
`--url`) in the docs. Reference the `agent-sync list` and `agent-sync sync`
command descriptions so the text stays consistent with the actual CLI behavior.
| def _fail_request(url: str, exc: httpx.HTTPError) -> None: | ||
| """Print a consistent daemon failure and exit. | ||
|
|
||
| Args: | ||
| url: Base server URL that failed. | ||
| exc: HTTPX exception raised by the request. | ||
|
|
||
| Raises: | ||
| SystemExit: Always exits with status 1. | ||
| """ | ||
| if isinstance(exc, httpx.HTTPStatusError): | ||
| status = exc.response.status_code | ||
| console.print(f"[red]Agent Sync API failed at {url}[/] (HTTP {status})") | ||
| if status in (404, 405): | ||
| console.print(" The running Hebb Mind service may be older than this checkout.") | ||
| console.print(" Restart the Hebb Mind service so CLI and server use the same version.") | ||
| else: | ||
| console.print(f" {exc}") | ||
| else: | ||
| console.print(f"[red]Cannot reach {url}[/]") | ||
| console.print(f" {exc}") | ||
| console.print(" Install/start the background service: [cyan]hebb service install[/]") | ||
| raise SystemExit(1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
_fail_request should be typed NoReturn, not None.
_fail_request always raises SystemExit(1) and is documented as such, but its signature is -> None. Under mypy strict, this means sessions (Line 33) and result (Line 52) will be reported as possibly-unbound after the try/except block, since mypy can't infer that control flow never returns from the except branch.
🐛 Proposed fix
-import json
+import json
from typing import Any
+from typing import NoReturn
...
-def _fail_request(url: str, exc: httpx.HTTPError) -> None:
+def _fail_request(url: str, exc: httpx.HTTPError) -> NoReturn:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _fail_request(url: str, exc: httpx.HTTPError) -> None: | |
| """Print a consistent daemon failure and exit. | |
| Args: | |
| url: Base server URL that failed. | |
| exc: HTTPX exception raised by the request. | |
| Raises: | |
| SystemExit: Always exits with status 1. | |
| """ | |
| if isinstance(exc, httpx.HTTPStatusError): | |
| status = exc.response.status_code | |
| console.print(f"[red]Agent Sync API failed at {url}[/] (HTTP {status})") | |
| if status in (404, 405): | |
| console.print(" The running Hebb Mind service may be older than this checkout.") | |
| console.print(" Restart the Hebb Mind service so CLI and server use the same version.") | |
| else: | |
| console.print(f" {exc}") | |
| else: | |
| console.print(f"[red]Cannot reach {url}[/]") | |
| console.print(f" {exc}") | |
| console.print(" Install/start the background service: [cyan]hebb service install[/]") | |
| raise SystemExit(1) | |
| import json | |
| from typing import Any | |
| from typing import NoReturn | |
| ... | |
| def _fail_request(url: str, exc: httpx.HTTPError) -> NoReturn: | |
| """Print a consistent daemon failure and exit. | |
| Args: | |
| url: Base server URL that failed. | |
| exc: HTTPX exception raised by the request. | |
| Raises: | |
| SystemExit: Always exits with status 1. | |
| """ | |
| if isinstance(exc, httpx.HTTPStatusError): | |
| status = exc.response.status_code | |
| console.print(f"[red]Agent Sync API failed at {url}[/] (HTTP {status})") | |
| if status in (404, 405): | |
| console.print(" The running Hebb Mind service may be older than this checkout.") | |
| console.print(" Restart the Hebb Mind service so CLI and server use the same version.") | |
| else: | |
| console.print(f" {exc}") | |
| else: | |
| console.print(f"[red]Cannot reach {url}[/]") | |
| console.print(f" {exc}") | |
| console.print(" Install/start the background service: [cyan]hebb service install[/]") | |
| raise SystemExit(1) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/cli/commands/agent_sync.py` around lines 204 - 226, _update
_fail_request in agent_sync.py to be annotated as NoReturn instead of None,
since it always exits via SystemExit(1) and never returns. Use the _fail_request
helper name and its existing exception-handling paths to update the return type,
and keep the raise SystemExit(1) behavior so mypy can correctly treat the except
branch as terminating and stop flagging sessions/result as possibly unbound._
Source: Coding guidelines
| for (session, turn, memory), embedding in zip(pending, embeddings, strict=True): | ||
| item = item_by_id[session.id] | ||
| try: | ||
| await store.create(memory, embedding=embedding) | ||
| except Exception: | ||
| item.failed += 1 | ||
| continue | ||
| item.memories_created += 1 | ||
| existing.add(session_sync.turn_key(session.host, session.session_id, turn.turn)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Silent failure on persist: no logging before incrementing failed.
Any exception from store.create() (including unexpected bugs, not just transient store errors) is swallowed with only a counter increment — there's no log trail to diagnose why turns failed to sync.
🪵 Proposed fix
+import logging
+
+logger = logging.getLogger(__name__)
+
...
try:
await store.create(memory, embedding=embedding)
except Exception:
+ logger.exception(
+ "Failed to persist synced turn host=%s session=%s turn=%s",
+ session.host, session.session_id, turn.turn,
+ )
item.failed += 1
continue📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (session, turn, memory), embedding in zip(pending, embeddings, strict=True): | |
| item = item_by_id[session.id] | |
| try: | |
| await store.create(memory, embedding=embedding) | |
| except Exception: | |
| item.failed += 1 | |
| continue | |
| item.memories_created += 1 | |
| existing.add(session_sync.turn_key(session.host, session.session_id, turn.turn)) | |
| import logging | |
| logger = logging.getLogger(__name__) | |
| for (session, turn, memory), embedding in zip(pending, embeddings, strict=True): | |
| item = item_by_id[session.id] | |
| try: | |
| await store.create(memory, embedding=embedding) | |
| except Exception: | |
| logger.exception( | |
| "Failed to persist synced turn host=%s session=%s turn=%s", | |
| session.host, session.session_id, turn.turn, | |
| ) | |
| item.failed += 1 | |
| continue | |
| item.memories_created += 1 | |
| existing.add(session_sync.turn_key(session.host, session.session_id, turn.turn)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/server/routers/agent_sync.py` around lines 122 - 130, The persist
path in the `agent_sync` loop silently swallows exceptions from
`store.create()`, so failures are impossible to diagnose. Update the
`try/except` around `store.create(memory, embedding=embedding)` to log the
exception with enough context (for example the current session/turn identifiers)
before incrementing `item.failed`; keep the counter update and `continue`, but
make sure the logger records the error path in the same `agent_sync` flow.
|
|
||
|
|
||
| def _source_checkout_command(module: str) -> list[str] | None: | ||
| """Return a source-checkout command when running from this repository. | ||
|
|
||
| Args: | ||
| module: Python module to execute with ``-m``. | ||
|
|
||
| Returns: | ||
| Command argv with ``PYTHONPATH`` pointed at the checkout's ``src`` | ||
| directory, or ``None`` when the current command is not being run from | ||
| this source tree. | ||
| """ | ||
| root = _source_checkout_root() | ||
| if root is None: | ||
| return None | ||
| src = root / "src" | ||
| python = _preferred_python(root) | ||
| if os.name == "nt": | ||
| return [str(python), "-m", module] | ||
| return ["/usr/bin/env", f"PYTHONPATH={src}", str(python), "-m", module] | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Windows branch drops PYTHONPATH, breaking the checkout-import contract.
The POSIX branch wraps the command with /usr/bin/env PYTHONPATH=<src> ... so the checkout's src is importable even without an editable install, but the Windows branch (os.name == "nt") returns [str(python), "-m", module] with no equivalent mechanism to set PYTHONPATH. If the selected .venv python doesn't already have the package installed (the exact scenario this feature targets), running on Windows will raise ModuleNotFoundError for module. The docstring's claim that the returned command has "PYTHONPATH pointed at the checkout's src directory" is also inaccurate for this branch. This gap isn't covered by tests either — the unit tests only monkeypatch os.name to "posix".
🩹 Proposed fix
if os.name == "nt":
- return [str(python), "-m", module]
+ # Windows has no /usr/bin/env-style inline env var trick; callers must
+ # merge PYTHONPATH into the subprocess env themselves, or use setx-style
+ # invocation. As a minimal fix, at least surface this via a documented
+ # convention, e.g. returning a tuple of (argv, env) or requiring callers
+ # to call a companion `_source_checkout_env(root)` helper.
+ return [str(python), "-m", module]Consider changing the return type to include the env override (e.g. tuple[list[str], dict[str, str]]) so both platforms propagate PYTHONPATH consistently, and updating callers/tests accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/utils/cli_paths.py` around lines 60 - 81, The Windows branch in
_source_checkout_command currently omits the PYTHONPATH override, so the source
checkout import behavior differs from the POSIX path; update the function to
propagate PYTHONPATH for both branches, likely by returning the command together
with an env override and adjusting callers accordingly. Make sure the docstring
for _source_checkout_command matches the actual behavior, and add tests that
cover the os.name == "nt" case so the checkout-import contract is verified on
Windows too.
d8ac982 to
149e9ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/hebb/static/css/style.css (1)
803-809: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
word-break: break-wordis deprecated.Per MDN,
break-wordis a deprecated legacy keyword with "the same effect as overflow-wrap: anywhere combined with word-break: normal, regardless of the actual value of the overflow-wrap property." Consider migrating to the modern equivalent.♻️ Suggested fix
.agent-flow-title, .agent-hub-name { font-size: 17px; font-weight: 700; color: var(--text-primary); - word-break: break-word; + overflow-wrap: anywhere; + word-break: normal; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/css/style.css` around lines 803 - 809, The title styles in agent-flow-title and agent-hub-name use the deprecated word-break: break-word value; update these rules to the modern equivalent by using overflow-wrap: anywhere together with word-break: normal so the text wrapping behavior stays the same while removing the legacy keyword.Source: Linters/SAST tools
tests/integration/server/test_agent_sync_router.py (1)
65-101: 🚀 Performance & Scalability | 🔵 TrivialDirect function calls skip the HTTP/DI layer despite the "integration" test label.
Both tests call
list_sessions/sync_sessionsdirectly with hand-rolled fakes rather than going through the FastAPI app (e.g., viaTestClient), so request validation, dependency wiring (store/embedderproviders), and JSON (de)serialization at the route boundary aren't exercised. If there's no other test that hits these endpoints over HTTP, consider adding one for full contract coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/server/test_agent_sync_router.py` around lines 65 - 101, The integration tests for list_sessions and sync_sessions are bypassing the FastAPI route layer by calling the functions directly, so they do not cover request validation, dependency injection, or JSON serialization at the HTTP boundary. Update these tests to exercise the actual app endpoints through the FastAPI test client, using the route handlers and their DI providers for store and embedder, so the contract is validated end to end.repo_pages/guide/web-console.md (1)
62-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a mermaid diagram for the Agent Sync data flow.
The section describes a cross-agent data flow (
Source software → Hebb Mind → Available to Claude Code / Codex) in prose only, while the rest of the page uses mermaid diagrams for architecture/data flow (see lines 19-33). As per coding guidelines, "Usemermaidfor architecture and data-flow diagrams (renders in VitePress and on GitHub)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@repo_pages/guide/web-console.md` around lines 62 - 72, Add a mermaid diagram to the Agent Sync section to represent the cross-agent data flow instead of leaving it only in prose. Update the content around the Agent Sync heading and the source-to-Hebb Mind-to-Claude Code/Codex flow so it matches the existing mermaid-based architecture diagrams used elsewhere on the page. Keep the surrounding bullets and CLI references, and ensure the new diagram clearly shows the sync path and available destinations.Source: Coding guidelines
src/hebb/cli/commands/agent_sync.py (1)
20-59: 📐 Maintainability & Code Quality | 🔵 TrivialAdd the required docstring sections to the public CLI callbacks.
agent_sync_cmd,list_cmd, andsync_cmdare public Python APIs, but their docstrings only have a summary line. The repo guideline requiresArgs,Returns, andRaisessections for public APIs.As per coding guidelines,
**/*.py: Public Python APIs must have docstrings that includeArgs,Returns, andRaises.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/cli/commands/agent_sync.py` around lines 20 - 59, The public CLI callbacks agent_sync_cmd, list_cmd, and sync_cmd only have summary docstrings, but they must follow the Python API docstring standard. Update each docstring to include Args for parameters like host, url, dry_run, and as_json, Returns for the command’s None return, and Raises for any expected click/httpx-related failures handled through _fail_request or propagated exceptions. Keep the existing symbols and behavior unchanged while expanding the docstrings to satisfy the repo guideline.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hebb/integrations/claude_code/transcript.py`:
- Around line 147-205: extract_turns currently lets OSError from
_load_main_messages escape, which can abort transcript processing instead of
skipping an unreadable file. Wrap the _load_main_messages call in extract_turns
with the same failure handling used by extract_last_turn, and return an empty
list or otherwise safely ignore the transcript when OSError occurs. Also update
the extract_turns docstring to include a Raises section documenting the OSError
behavior, keeping it consistent with the public API guidelines.
---
Nitpick comments:
In `@repo_pages/guide/web-console.md`:
- Around line 62-72: Add a mermaid diagram to the Agent Sync section to
represent the cross-agent data flow instead of leaving it only in prose. Update
the content around the Agent Sync heading and the source-to-Hebb Mind-to-Claude
Code/Codex flow so it matches the existing mermaid-based architecture diagrams
used elsewhere on the page. Keep the surrounding bullets and CLI references, and
ensure the new diagram clearly shows the sync path and available destinations.
In `@src/hebb/cli/commands/agent_sync.py`:
- Around line 20-59: The public CLI callbacks agent_sync_cmd, list_cmd, and
sync_cmd only have summary docstrings, but they must follow the Python API
docstring standard. Update each docstring to include Args for parameters like
host, url, dry_run, and as_json, Returns for the command’s None return, and
Raises for any expected click/httpx-related failures handled through
_fail_request or propagated exceptions. Keep the existing symbols and behavior
unchanged while expanding the docstrings to satisfy the repo guideline.
In `@src/hebb/static/css/style.css`:
- Around line 803-809: The title styles in agent-flow-title and agent-hub-name
use the deprecated word-break: break-word value; update these rules to the
modern equivalent by using overflow-wrap: anywhere together with word-break:
normal so the text wrapping behavior stays the same while removing the legacy
keyword.
In `@tests/integration/server/test_agent_sync_router.py`:
- Around line 65-101: The integration tests for list_sessions and sync_sessions
are bypassing the FastAPI route layer by calling the functions directly, so they
do not cover request validation, dependency injection, or JSON serialization at
the HTTP boundary. Update these tests to exercise the actual app endpoints
through the FastAPI test client, using the route handlers and their DI providers
for store and embedder, so the contract is validated end to end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f23cf9b9-34b5-4033-b780-428f6c5a301b
📒 Files selected for processing (37)
AGENTS.mdREADME.mdREADME_ZH.mdrepo_pages/.vitepress/config.mtsrepo_pages/api/cli.mdrepo_pages/guide/agent-sync.mdrepo_pages/guide/web-console.mdrepo_pages/index.mdrepo_pages/public/llms.txtrepo_pages/quick-start.mdrepo_pages/zh/api/cli.mdrepo_pages/zh/guide/agent-sync.mdrepo_pages/zh/guide/web-console.mdrepo_pages/zh/index.mdrepo_pages/zh/quick-start.mdreports/analysis/codex-claude-code-session-memory-analysis.mdreports/design/agent-session-sync-design.mdsrc/hebb/cli/commands/agent_sync.pysrc/hebb/cli/main.pysrc/hebb/integrations/claude_code/transcript.pysrc/hebb/integrations/codex/transcript.pysrc/hebb/integrations/session_sync.pysrc/hebb/server/app.pysrc/hebb/server/routers/agent_sync.pysrc/hebb/static/css/style.csssrc/hebb/static/index.htmlsrc/hebb/static/js/api.jssrc/hebb/static/js/app.jssrc/hebb/static/js/components/agent-sync.jssrc/hebb/static/js/i18n.jssrc/hebb/utils/cli_paths.pytests/integration/server/test_agent_sync_router.pytests/unit/cli/commands/test_agent_sync.pytests/unit/integrations/test_agent_session_sync.pytests/unit/integrations/test_claude_code_hooks.pytests/unit/integrations/test_codex_hooks.pytests/unit/utils/test_cli_paths.py
✅ Files skipped from review due to trivial changes (10)
- repo_pages/guide/agent-sync.md
- README_ZH.md
- reports/analysis/codex-claude-code-session-memory-analysis.md
- src/hebb/static/js/i18n.js
- repo_pages/zh/api/cli.md
- repo_pages/quick-start.md
- repo_pages/api/cli.md
- README.md
- repo_pages/zh/index.md
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (15)
- repo_pages/public/llms.txt
- repo_pages/.vitepress/config.mts
- repo_pages/index.md
- tests/unit/integrations/test_codex_hooks.py
- src/hebb/static/js/api.js
- tests/unit/utils/test_cli_paths.py
- src/hebb/cli/main.py
- src/hebb/server/app.py
- src/hebb/static/index.html
- src/hebb/utils/cli_paths.py
- src/hebb/static/js/app.js
- tests/unit/cli/commands/test_agent_sync.py
- src/hebb/integrations/codex/transcript.py
- src/hebb/integrations/session_sync.py
- src/hebb/server/routers/agent_sync.py
| def extract_turns(transcript_path: str | Path) -> list[TurnRecord]: | ||
| """Extract all complete user-to-assistant turns from a Claude Code JSONL transcript. | ||
|
|
||
| Args: | ||
| transcript_path: Path to the session ``.jsonl`` file. | ||
|
|
||
| Returns: | ||
| Parsed turn records in transcript order. Low-signal user prompts and | ||
| incomplete turns without assistant output are omitted. | ||
| """ | ||
| messages = _load_main_messages(Path(transcript_path)) | ||
| if not messages: | ||
| return [] | ||
|
|
||
| human_indices = [i for i, msg in enumerate(messages) if msg.get("type") == "user" and _raw_user_text(msg)] | ||
| records: list[TurnRecord] = [] | ||
|
|
||
| for pos, user_idx in enumerate(human_indices): | ||
| user_msg = messages[user_idx] | ||
| user_text = _extract_user_text(user_msg) | ||
| if not user_text: | ||
| continue | ||
|
|
||
| next_user_idx = human_indices[pos + 1] if pos + 1 < len(human_indices) else len(messages) | ||
| segment = messages[user_idx + 1 : next_user_idx] | ||
|
|
||
| summary = TurnSummary(user_input=user_text, turn=pos) | ||
| for msg in segment: | ||
| if msg.get("type") == "assistant": | ||
| _extract_assistant(msg, summary, text=False) | ||
|
|
||
| for msg in reversed(segment): | ||
| if msg.get("type") == "assistant": | ||
| candidate = TurnSummary() | ||
| _extract_assistant(msg, candidate, text=True) | ||
| if candidate.assistant_output: | ||
| summary.assistant_output = candidate.assistant_output | ||
| break | ||
|
|
||
| summary.tools = _dedup(summary.tools) | ||
| summary.mcps = _dedup(summary.mcps) | ||
| if not summary.assistant_output: | ||
| continue | ||
|
|
||
| timestamp = user_msg.get("timestamp") | ||
| session_id = user_msg.get("sessionId") or user_msg.get("session_id") | ||
| cwd = user_msg.get("cwd") | ||
| records.append( | ||
| TurnRecord( | ||
| summary=summary, | ||
| timestamp=timestamp if isinstance(timestamp, str) else None, | ||
| session_id=session_id if isinstance(session_id, str) else None, | ||
| cwd=cwd if isinstance(cwd, str) else None, | ||
| ) | ||
| ) | ||
|
|
||
| return records | ||
|
|
||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
extract_turns doesn't handle OSError from _load_main_messages; missing Raises docstring section.
_load_main_messages documents (and does) re-raise OSError when a transcript exists but can't be read (lines 308-309, 331-333). extract_last_turn guards against this (lines 85-88, returns None), but extract_turns calls _load_main_messages directly at line 157 with no try/except. A single unreadable/permission-denied transcript will now raise uncaught through extract_claude_turns in session_sync.py, aborting the whole Agent Sync discovery batch rather than just skipping that file — inconsistent with the sibling function's failure mode.
The docstring at lines 147-156 also omits the Raises section required by project guidelines for public APIs.
🐛 Proposed fix
Returns:
Parsed turn records in transcript order. Low-signal user prompts and
incomplete turns without assistant output are omitted.
+
+ Raises:
+ Nothing — read failures are caught and result in an empty list.
"""
- messages = _load_main_messages(Path(transcript_path))
+ try:
+ messages = _load_main_messages(Path(transcript_path))
+ except OSError:
+ return []
if not messages:
return []As per coding guidelines, "Include docstring with Args, Returns, and Raises sections for all public APIs."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def extract_turns(transcript_path: str | Path) -> list[TurnRecord]: | |
| """Extract all complete user-to-assistant turns from a Claude Code JSONL transcript. | |
| Args: | |
| transcript_path: Path to the session ``.jsonl`` file. | |
| Returns: | |
| Parsed turn records in transcript order. Low-signal user prompts and | |
| incomplete turns without assistant output are omitted. | |
| """ | |
| messages = _load_main_messages(Path(transcript_path)) | |
| if not messages: | |
| return [] | |
| human_indices = [i for i, msg in enumerate(messages) if msg.get("type") == "user" and _raw_user_text(msg)] | |
| records: list[TurnRecord] = [] | |
| for pos, user_idx in enumerate(human_indices): | |
| user_msg = messages[user_idx] | |
| user_text = _extract_user_text(user_msg) | |
| if not user_text: | |
| continue | |
| next_user_idx = human_indices[pos + 1] if pos + 1 < len(human_indices) else len(messages) | |
| segment = messages[user_idx + 1 : next_user_idx] | |
| summary = TurnSummary(user_input=user_text, turn=pos) | |
| for msg in segment: | |
| if msg.get("type") == "assistant": | |
| _extract_assistant(msg, summary, text=False) | |
| for msg in reversed(segment): | |
| if msg.get("type") == "assistant": | |
| candidate = TurnSummary() | |
| _extract_assistant(msg, candidate, text=True) | |
| if candidate.assistant_output: | |
| summary.assistant_output = candidate.assistant_output | |
| break | |
| summary.tools = _dedup(summary.tools) | |
| summary.mcps = _dedup(summary.mcps) | |
| if not summary.assistant_output: | |
| continue | |
| timestamp = user_msg.get("timestamp") | |
| session_id = user_msg.get("sessionId") or user_msg.get("session_id") | |
| cwd = user_msg.get("cwd") | |
| records.append( | |
| TurnRecord( | |
| summary=summary, | |
| timestamp=timestamp if isinstance(timestamp, str) else None, | |
| session_id=session_id if isinstance(session_id, str) else None, | |
| cwd=cwd if isinstance(cwd, str) else None, | |
| ) | |
| ) | |
| return records | |
| def extract_turns(transcript_path: str | Path) -> list[TurnRecord]: | |
| """Extract all complete user-to-assistant turns from a Claude Code JSONL transcript. | |
| Args: | |
| transcript_path: Path to the session ``.jsonl`` file. | |
| Returns: | |
| Parsed turn records in transcript order. Low-signal user prompts and | |
| incomplete turns without assistant output are omitted. | |
| Raises: | |
| Nothing — read failures are caught and result in an empty list. | |
| """ | |
| try: | |
| messages = _load_main_messages(Path(transcript_path)) | |
| except OSError: | |
| return [] | |
| if not messages: | |
| return [] | |
| human_indices = [i for i, msg in enumerate(messages) if msg.get("type") == "user" and _raw_user_text(msg)] | |
| records: list[TurnRecord] = [] | |
| for pos, user_idx in enumerate(human_indices): | |
| user_msg = messages[user_idx] | |
| user_text = _extract_user_text(user_msg) | |
| if not user_text: | |
| continue | |
| next_user_idx = human_indices[pos + 1] if pos + 1 < len(human_indices) else len(messages) | |
| segment = messages[user_idx + 1 : next_user_idx] | |
| summary = TurnSummary(user_input=user_text, turn=pos) | |
| for msg in segment: | |
| if msg.get("type") == "assistant": | |
| _extract_assistant(msg, summary, text=False) | |
| for msg in reversed(segment): | |
| if msg.get("type") == "assistant": | |
| candidate = TurnSummary() | |
| _extract_assistant(msg, candidate, text=True) | |
| if candidate.assistant_output: | |
| summary.assistant_output = candidate.assistant_output | |
| break | |
| summary.tools = _dedup(summary.tools) | |
| summary.mcps = _dedup(summary.mcps) | |
| if not summary.assistant_output: | |
| continue | |
| timestamp = user_msg.get("timestamp") | |
| session_id = user_msg.get("sessionId") or user_msg.get("session_id") | |
| cwd = user_msg.get("cwd") | |
| records.append( | |
| TurnRecord( | |
| summary=summary, | |
| timestamp=timestamp if isinstance(timestamp, str) else None, | |
| session_id=session_id if isinstance(session_id, str) else None, | |
| cwd=cwd if isinstance(cwd, str) else None, | |
| ) | |
| ) | |
| return records |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/claude_code/transcript.py` around lines 147 - 205,
extract_turns currently lets OSError from _load_main_messages escape, which can
abort transcript processing instead of skipping an unreadable file. Wrap the
_load_main_messages call in extract_turns with the same failure handling used by
extract_last_turn, and return an empty list or otherwise safely ignore the
transcript when OSError occurs. Also update the extract_turns docstring to
include a Raises section documenting the OSError behavior, keeping it consistent
with the public API guidelines.
Source: Coding guidelines
1 participant
Summary
hebb agent-syncCLI parity plus public EN/ZH docs and internal design notesTesting
uv run ruff check srcuv run mypy src/hebb/uv run pytest tests/ -q --tb=shortnpm run docs:buildfromrepo_pagesSummary by CodeRabbit
New Features
Documentation
Bug Fixes